Rich Text Editor | Implementation of additional APIs #1446
Conversation
…rs/vivin/integrate-tiptap-editor
…rs/vivin/integrate-tiptap-editor
…rs/vivin/integrate-tiptap-editor
…rs/vivin/integrate-tiptap-editor
…rs/vivin/integrate-tiptap-editor
… into separate PRs
| --ni-private-rich-text-editor-footer-section-height: 40px; | ||
| ${ | ||
| /** Minimum width is added to accommodate all the possible buttons in the toolbar and to support the mobile width. */ '' | ||
| } | ||
| min-width: 360px; | ||
| } | ||
|
|
||
| :host([fit-to-content]) { | ||
| max-height: inherit; |
There was a problem hiding this comment.
All of these inherits on sizing is a big red flag to me. You pretty much never want to inherit on dimensions. See an example here: #1416 (comment)
This example shows how things can go wrong. Here the user set a width on the editor of 500px and made it position fixed where it is intended to be positioned and sized unrealted to the parent. But the editor had default host styling of inherit. It caused the editor to unexpectedly inherit a width from an unrelated parent element: https://jsbin.com/feripobimu/2/edit?html,js,output
There was a problem hiding this comment.
I have a feeling based on this feedback and the discussions in the other threads that the style updates needed for fit-to-content may require a few more rounds of feedback.
You may want to consider splitting it into a separate PR but totally up to you.
There was a problem hiding this comment.
@rajsite @jattasNI Due to a time constraint to integrate this component into SLE, which relies entirely on this PR and the angular support PR, we have decided to branch out the fit-to-content related changes into a separate PR as suggested.
We will address all the comments provided for fit-to-content in the follow-up PR which will be created right after we come up with a better implementation approach and after this PR goes in. Therefore, I have removed all the changes related to fit-to-content and other style changes related to inherit from this PR including storybook docs, storybook tests, etc,..
Please note that I will also be removing the fit-to-content related changes from the Angular PR (#1450) which is already approved by every reviewer and will be added along with the follow-up PR.
packages/nimble-components/src/rich-text-editor/testing/rich-text-editor.pageobject.ts
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor/testing/rich-text-editor.pageobject.ts
Outdated
Show resolved
Hide resolved
packages/nimble-components/src/rich-text-editor/tests/rich-text-editor.stories.ts
Outdated
Show resolved
Hide resolved
… the follow up PR.
|
I decline to review. |
1 similar comment
|
I decline to review. |
packages/nimble-components/src/rich-text-editor/tests/rich-text-editor-matrix.stories.ts
Show resolved
Hide resolved
Pull Request
🤨 Rationale
See the API section of the spec.
👩💻 Implementation
disabled- boolean attribute when enabled, set the Tiptap's editable state to false through setEditable() and set thedisabledattribute of "nimble-toggle-button".footer-hidden- hide the footer through styles.placeholder- installed Tiptap's Placeholder extension and configured the styles to show the placeholder text when the editor is empty.empty- read-only property that returns "true" if the editor is empty by getting the current "textContent" of the editor.error-visible- styled the editor with error color and added an exclamation icon to the editor same as in the "nimble-textarea". Used the "ResizeObserver" to update the scrollbar width and position of the error icon in the editor accordingly.error-text- shows the text in the bottom left of the editor just like other nimble components.@tiptapextensions to the latest version.🧪 Testing
✅ Checklist